Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide file-picker navigation #6116

Closed
wants to merge 1 commit into from

Conversation

paholg
Copy link

@paholg paholg commented Feb 27, 2023

We provide the following new keybindings while in a picker:

  • C-e - Change picker root to one directory in, based on selection
  • C-a - Change picker root to one directory out

These can be especially useful when combined with the command
file_picker_in_current_buffer_directory when navigating library
files.

That is, with this change the following flow is enabled:

  1. Perform a goto_defition on a symbol defined in an external library.
  2. Perform file_picker_in_current_buffer_directory, opening a picker
    in the external library.
  3. Browse the external library with C-e and C-a, which is not
    possible without this change.

To accomplish this, we need access to a FilePickerConfig not just
when building the picker, but also later, so we add an associated
Item::Config type, which is simply () for everything except
PathBuf.

To make it easier to follow what is going on when navigating the
picker, we set the status bar to the picker's root directory on change.
If #8155 is merged, we can use that instead of changing the status bar.

Comment on lines 416 to 417
| `Ctrl-l` | Move file picker to parent directory |
| `Ctrl-o` | Narrow file picker on selected directory |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does these keys comes from? What's the prior art? Why l and o and not i and o or something else?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prior art is emacs's helm-mode, which uses ctrl-l for a similar purpose, and I somewhat arbitrarily picked ctrl-o because it was nearby and not taken.

I would be happy to change these commands to something more intuitive. Do you think ctrl-o to "move out" and ctrl-i to "move in" would be the best?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we cannot use ctrl-i as the terminal sends that as tab, which is already bound.

Instead, I have opted for ctrl-e and ctrl-a as these already have meaning for going forward and backward, though I'm happy to change to something else.

@paholg paholg force-pushed the file-picker-navigation branch 2 times, most recently from 1cbdbf6 to 617ac4d Compare September 21, 2023 06:11
@paholg
Copy link
Author

paholg commented Sep 21, 2023

I find this commit necessary for using helix, as there's no real way to navigate external libraries otherwise. I imagine others would find it useful as well, so would definitely appreciate a review.

@perlinm

This comment has been minimized.

@thomasaarholt
Copy link
Contributor

I just tested this PR, and the functionality in it seems useful and works as advertised. It seems a little strange to navigate up and down directories while in a fuzzy file picker, but I think that is mainly due to it being a new concept to me.

I think that this functionality can happily coexist next to a file explorer ala #5768, that is, even if we had a file explorer in helix today, this PR would still be desirable.

@paholg paholg force-pushed the file-picker-navigation branch 3 times, most recently from 0f7d9ec to efeac87 Compare February 6, 2024 20:27
@paholg paholg force-pushed the file-picker-navigation branch 2 times, most recently from b589d07 to 2225190 Compare March 14, 2024 04:19
@paholg paholg force-pushed the file-picker-navigation branch 2 times, most recently from 22600b1 to 70197eb Compare March 23, 2024 14:31
@kirawi
Copy link
Member

kirawi commented Mar 29, 2024

As I mentioned in Matrix, there was some talk a while ago about a basic file explorer:

A filepicker with depth 1 that shows directories and if you select a directory it changes directories to that picker. Add a key to go to go up a directory and make the location of the picker persist across closing/opening it and you got yourself so.ething close to a filetree without the need for a new ui component

It might be worth pivoting this PR towards something like that. But AFAIK this isn't a solid plan and you might want to wait for input from the maintainers on this.

@kirawi kirawi added S-waiting-on-review Status: Awaiting review from a maintainer. A-command Area: Commands labels Mar 29, 2024
@paholg paholg force-pushed the file-picker-navigation branch from 70197eb to 37fdce5 Compare April 1, 2024 15:37
We provide the following new keybindings while in a picker:
* `C-e` - Change picker root to one directory in, based on selection
* `C-a` - Change picker root to one directory out

These can be especially useful when combined with the command
`file_picker_in_current_buffer_directory` when navigating library
files.

That is, with this change the following flow is enabled:
1. Perform a `goto_defition` on a symbol defined in an external library.
2. Perform `file_picker_in_current_buffer_directory`, opening a picker
   in the external library.
3. Browse the external library with `C-e` and `C-a`, which is not
   possible without this change.

To accomplish this, we need access to a `FilePickerConfig` not just
when building the picker, but also later, so we add an associated
`Item::Config` type, which is simply `()` for everything except
`PathBuf`.

To make it easier to follow what is going on when navigating the
picker, we set the status bar to the picker's root directory on change.
@paholg paholg force-pushed the file-picker-navigation branch from 37fdce5 to 05635f4 Compare April 1, 2024 16:07
@paholg
Copy link
Author

paholg commented Apr 1, 2024

That's interesting. Honestly, for me, this implementation is more useful; I still want to see everything when navigating up and down directories.

But if adding that functionality would help get this reviewed, I'd be happy to take a stab at it.

@pascalkuthe
Copy link
Member

closing this one as stale. I don't think we will go forward with these bindings. They could be added as alternatives when we have remappable component key maps available but they may also be a bit niche for core and better suited for a scribtable config or a plugin. Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants